Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.4] Able to use multiple Redis cluster and non-cluster instances together #16696

Merged
merged 5 commits into from
Dec 12, 2016
Merged

[5.4] Able to use multiple Redis cluster and non-cluster instances together #16696

merged 5 commits into from
Dec 12, 2016

Conversation

alibo
Copy link
Contributor

@alibo alibo commented Dec 7, 2016

This PR allows user to create and use multiple Redis cluster as well as non-cluster instances.

Sample configs:

  1. Multiple Redis clusters:
    'redis' => [
        'clusters' => [
            'redis-cluster-1' => [
                [
                    'host'     => '127.0.0.1',
                    'port'     => 6379,
                    'database' => 0,
                ],
                [
                    'host'     => '127.0.0.1',
                    'port'     => 6380,
                    'database' => 0,
                ],
            ],
            'redis-cluster-2' => [
                [
                    'host'     => '127.0.0.1',
                    'port'     => 7379,
                    'database' => 0,
                ],
                [
                    'host'     => '127.0.0.1',
                    'port'     => 7380,
                    'database' => 0,
                ],
            ],
        ],
    ],
  1. Multiple Redis clusters with different options:
    'redis' => [
        'clusters' => [
            'redis-cluster-1' => [
                'options' => [
                    'cluster' => 'redis',
                ],
                [
                    'host'     => '127.0.0.1',
                    'port'     => 6379,
                    'database' => 0,
                ],
                [
                    'host'     => '127.0.0.1',
                    'port'     => 6380,
                    'database' => 0,
                ],
            ],
            'redis-cluster-2' => [
                'options' => [
                    'cluster' => 'predis',
                ],
                [
                    'host'     => '127.0.0.1',
                    'port'     => 7379,
                    'database' => 0,
                ],
                [
                    'host'     => '127.0.0.1',
                    'port'     => 7380,
                    'database' => 0,
                ],
            ],
        ],
    ],
  1. Single Redis cluster and two non-cluster instances:
    'redis' => [
        'default' => [
            'host' => '127.0.0.1',
            'port' => 16379,
            'database' => 0,
        ],
        'queue' => [
            'host' => '127.0.0.1',
            'port' => 26379,
            'database' => 0,
        ],
        'clusters' => [
            'redis-cluster' => [
                [
                    'host'     => '127.0.0.1',
                    'port'     => 6379,
                    'database' => 0,
                ],
                [
                    'host'     => '127.0.0.1',
                    'port'     => 6380,
                    'database' => 0,
                ],
            ],
        ],
    ],
  1. It won't break anything! (unless you're using clusters as a key!)
    (Maybe we can deprecate cluster option after merging this PR?)
   'redis' => [
        'cluster' => true,
        'default' => [
            'host' => '127.0.0.1',
            'port' => 16379,
            'database' => 0,
        ],
        'clusters' => [
            'redis-cluster-1' => [
                [
                    'host'     => '127.0.0.1',
                    'port'     => 6379,
                    'database' => 0,
                ]
            ],
            'redis-cluster-2' => [
                [
                    'host'     => '127.0.0.1',
                    'port'     => 7379,
                    'database' => 0,
                ]
            ],
        ],
    ],

With above config, we have 3 connections which all of them are clusters: default, redis-cluster-1, redis-cluster-2


I also add some tests for PhpRedis. They're not good but better than nothing!! If you can make them better, Please tell me or of course you can send an another PR :D

@alibo alibo changed the title Able to use multiple Redis cluster and non-cluster instances together [5.4] Able to use multiple Redis cluster and non-cluster instances together Dec 7, 2016
@alibo
Copy link
Contributor Author

alibo commented Dec 8, 2016

Yeah finally! I fixed the cs :))

I don't know why this command is not working, it's just freezing!

$ php-cs-fixer fix "src/Illuminate/Redis" --config-file=".php_cs"
Loaded config from ".php_cs".

@GrahamCampbell
Copy link
Member

I don't know why this command is not working, it's just freezing!

I wouldn't recommend using php-cs-fixer locally. Just apply the diff StyleCI gives you, or, just don't bother. StyleCI will merge its own fixes after the PR is merged.

@alibo
Copy link
Contributor Author

alibo commented Dec 8, 2016

@GrahamCampbell Thank you :) I didn't know StyleCI also can fix the code style. 👍

@GrahamCampbell
Copy link
Member

Please revert all the bad changes you've made to the phpdoc.

@alibo
Copy link
Contributor Author

alibo commented Dec 10, 2016

@GrahamCampbell phpstorm 😶
It's ok now :)

@taylorotwell
Copy link
Member

Had someone raise another PR about Redis Cluster 3.0. Does this work with Redis Cluster 3.0?

@alibo
Copy link
Contributor Author

alibo commented Dec 12, 2016

@taylorotwell I didn't change those PhpRedis codes. With this PR, you just can connect to multiple Redis cluster instances, as well as, non-cluster instances at the same time. Before this, you can only have multiple non-cluster instances or just a single Redis cluster instance. ('cluster' => true|false)


But I've tested them and both Predis and RedisCluster work:

3 masters and 3 slaves (based on https://redis.io/topics/cluster-tutorial):
7000 7001 7002 7003 7004 7005

Configuring:

nano ~/redis-3.2.6/cluster-test/700{0..5}/redis.conf

port 7000 #7000-7005
cluster-enabled yes
cluster-config-file nodes.conf
cluster-node-timeout 5000
appendonly yes

Running redis servers:

cd ~/redis-3.2.6/cluster-test/7000 && ../../src/redis-server redis.conf
cd ~/redis-3.2.6/cluster-test/7001 && ../../src/redis-server redis.conf
cd ~/redis-3.2.6/cluster-test/7002 && ../../src/redis-server redis.conf
cd ~/redis-3.2.6/cluster-test/7003 && ../../src/redis-server redis.conf
cd ~/redis-3.2.6/cluster-test/7004 && ../../src/redis-server redis.conf
cd ~/redis-3.2.6/cluster-test/7005 && ../../src/redis-server redis.conf

Creating cluster:

./redis-trib.rb create --replicas 1 127.0.0.1:7000 127.0.0.1:7001 \
 ◇ 127.0.0.1:7002 127.0.0.1:7003 127.0.0.1:7004 127.0.0.1:7005

Cluster info:

Using 3 masters:
127.0.0.1:7000
127.0.0.1:7001
127.0.0.1:7002
Adding replica 127.0.0.1:7003 to 127.0.0.1:7000
Adding replica 127.0.0.1:7004 to 127.0.0.1:7001
Adding replica 127.0.0.1:7005 to 127.0.0.1:7002
M: 805950d657684787324c4865a4521079c648dee8 127.0.0.1:7000
   slots:0-5460 (5461 slots) master
M: cc311766ed77787dafb5dad6dca804a6d5ae774a 127.0.0.1:7001
   slots:5461-10922 (5462 slots) master
M: 5c2505e4f54c7ea1750300cab62287c2bb6c19b1 127.0.0.1:7002
   slots:10923-16383 (5461 slots) master
S: ca4364e034e089069ba01c09d14478e33c822eef 127.0.0.1:7003
   replicates 805950d657684787324c4865a4521079c648dee8
S: 5288746230933323e00c618979cf6e1ee11679d8 127.0.0.1:7004
   replicates cc311766ed77787dafb5dad6dca804a6d5ae774a
S: 86de908e51e7757431d6e42337024f69c60bebd7 127.0.0.1:7005
   replicates 5c2505e4f54c7ea1750300cab62287c2bb6c19b1

Laravel Redis Config (Predis):

   'redis' => [

        'client' => 'predis',

        'cluster' => false,

        'clusters' =>[ 
            'cluster-1' => [
                'options' => [
                    'cluster' => 'redis',
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7000,
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7001,
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7002,
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7003,
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7004,
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7005,
                ],
            ]
        ]

    ],

Laravel Redis Config (PhpRedis):

  'redis' => [

        'client' => 'phpredis',

        'cluster' => false,

        'clusters' =>[ 
            'cluster-1' => [
                [
                    'host' => '127.0.0.1',
                    'port' => 7000,
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7001,
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7002,
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7003,
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7004,
                ],
                [
                    'host' => '127.0.0.1',
                    'port' => 7005,
                ],
            ]
        ]

    ],

My php version:

PHP 7.1.0 (cli) (built: Dec  2 2016 03:30:24) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.1.0-dev, Copyright (c) 1998-2016 Zend Technologies

Testing Predis cluster:

art tinker
Psy Shell v0.7.2 (PHP 7.1.0 — cli) by Justin Hileman
>>> $redis = app('redis');
=> Illuminate\Redis\PredisDatabase {#649}
>>> dump($redis->connection('cluster-1')->set('testing-1', 1));
Predis\Response\Status {#678
  -payload: "OK"
}
=> null
>>> dump($redis->connection('cluster-1')->set('testing-2', 2));
Predis\Response\Status {#678
  -payload: "OK"
}
=> null
>>> dump($redis->connection('cluster-1')->set('testing-3', 3));
Predis\Response\Status {#678
  -payload: "OK"
}
=> null
redis-cli -p 7000 -c
127.0.0.1:7000> get testing-1
-> Redirected to slot [12279] located at 127.0.0.1:7002
"1"
127.0.0.1:7002> get testing-2
-> Redirected to slot [8084] located at 127.0.0.1:7001
"2"
127.0.0.1:7001> get testing-3
-> Redirected to slot [4021] located at 127.0.0.1:7000
"3"

Testing PhpRedis cluster:

art tinker
Psy Shell v0.7.2 (PHP 7.1.0 — cli) by Justin Hileman
>>> $redis = app('redis');
=> Illuminate\Redis\PhpRedisDatabase {#649}
>>> dump($redis->connection('cluster-1')->set('testing-phpredis-1', 1));
true
=> null
>>> dump($redis->connection('cluster-1')->set('testing-phpredis-2', 2));
true
=> null
>>> dump($redis->connection('cluster-1')->set('testing-phpredis-3', 3));
true
=> null
redis-cli -p 7000 -c
127.0.0.1:7000> get testing-phpredis-1
-> Redirected to slot [15531] located at 127.0.0.1:7002
"1"
127.0.0.1:7002> get testing-phpredis-2
-> Redirected to slot [3272] located at 127.0.0.1:7000
"2"
127.0.0.1:7000> get testing-phpredis-3
-> Redirected to slot [7401] located at 127.0.0.1:7001
"3"
127.0.0.1:7001>

@taylorotwell
Copy link
Member

So this is different from the current "cluster" option right? That option just did client side sharding which is not the true official "Redis Cluster", right? This now adds support for the actual official Redis Cluster stuff, correct?

@taylorotwell taylorotwell merged commit cd63104 into laravel:master Dec 12, 2016
@taylorotwell
Copy link
Member

taylorotwell commented Dec 12, 2016

I've read through this and cleaned up a few things and got it merged. I am going to deprecate the "cluster" option in 5.4 and just use the clusters array introduced here instead.

@alibo
Copy link
Contributor Author

alibo commented Dec 12, 2016

That option just did client side sharding which is not the true official "Redis Cluster", right?

@taylorotwell Laravel already has true official "Redis Cluster". You should only add this to Redis config array:

'options' => [
    'cluster' => 'redis'
]

More info: https://github.com/nrk/predis#cluster and https://github.com/nrk/predis/wiki/Client-Options

This now adds support for the actual official Redis Cluster stuff, correct?

No, it allows you to use multiple Redis clusters. Also, you can use non-cluster instances for something else such as Redis geo, queue, etc. (Redis cluster has own limitations!)
Before this PR, if you set cluster to true, you only have one single cluster connection (default), without any non-cluster instances.

I am going to deprecate the "cluster" option in 5.4 and just use the clusters array introduced here instead.

Perfect 👌 Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants